Skip to content

Remove unused methods of OC_Response#8792

Merged
MorrisJobke merged 2 commits intomasterfrom
cleanup-oc_response
Mar 13, 2018
Merged

Remove unused methods of OC_Response#8792
MorrisJobke merged 2 commits intomasterfrom
cleanup-oc_response

Conversation

@MorrisJobke
Copy link
Member

Ref #8375 and #7827

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke MorrisJobke added 3. to review Waiting for reviews technical debt 🧱 🤔🚀 labels Mar 12, 2018
@MorrisJobke MorrisJobke added this to the Nextcloud 14 milestone Mar 12, 2018
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good otherwise

if($token !== '') {
OC_Response::redirect($urlGenerator->linkToRoute($route, array('token' => $token)));
$protocol = \OC::$server->getRequest()->getHttpProtocol();
if ($protocol == 'HTTP/1.1') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

===?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a copy paste and should be replaced soon anyways.

if ($protocol == 'HTTP/1.1') {
$status = '307 Temporary Redirect';
} else {
$status = '304 Found';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

304 Not Modified is the proper code?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or 302 Found? :)

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@codecov
Copy link

codecov bot commented Mar 13, 2018

Codecov Report

Merging #8792 into master will decrease coverage by 0.05%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #8792      +/-   ##
============================================
- Coverage     51.87%   51.81%   -0.06%     
- Complexity    25358    25379      +21     
============================================
  Files          1607     1607              
  Lines         95101    95087      -14     
  Branches       1377     1377              
============================================
- Hits          49336    49272      -64     
- Misses        45765    45815      +50
Impacted Files Coverage Δ Complexity Δ
lib/private/legacy/response.php 0% <ø> (ø) 16 <0> (-20) ⬇️
lib/public/Response.php 0% <ø> (ø) 2 <0> (-7) ⬇️
apps/files_sharing/public.php 0% <0%> (ø) 0 <0> (ø) ⬇️
lib/private/Files/ObjectStore/Swift.php 0% <0%> (-75%) 8% <0%> (ø)
lib/private/Files/ObjectStore/SwiftFactory.php 0% <0%> (-52.88%) 35% <0%> (ø)
lib/public/DB.php 26.66% <0%> (-13.34%) 8% <0%> (+3%)
lib/private/Tags.php 47.98% <0%> (-4.99%) 152% <0%> (+45%)

@MorrisJobke
Copy link
Member Author

Thanks @skjnldsv was wrong status code in the initial code. I also fixed the HTTP/1.0 detection ;)

@rullzer
Copy link
Member

rullzer commented Mar 13, 2018

HTTP 1.0 support needs to die anyway. 1.1 was release in 1997

@skjnldsv
Copy link
Member

Cant we return HTTP 418 instead? :D

@MorrisJobke MorrisJobke merged commit 7488218 into master Mar 13, 2018
@MorrisJobke MorrisJobke deleted the cleanup-oc_response branch March 13, 2018 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews technical debt 🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants